Skip to content

Fix -Wconversion warnings from gcc 8.5.0#4051

Merged
kddnewton merged 2 commits intoruby:mainfrom
eregon:fix-gcc85-conversion-warnings
Mar 27, 2026
Merged

Fix -Wconversion warnings from gcc 8.5.0#4051
kddnewton merged 2 commits intoruby:mainfrom
eregon:fix-gcc85-conversion-warnings

Conversation

@eregon
Copy link
Copy Markdown
Member

@eregon eregon commented Mar 27, 2026

Fixes #4048

@eregon eregon requested review from Earlopain and kddnewton March 27, 2026 13:57
@eregon
Copy link
Copy Markdown
Member Author

eregon commented Mar 27, 2026

I'd be interested to add a CI job running in almalinux:8 to check this, based on https://github.com/truffleruby/truffleruby/blob/master/tool/build-with-old-glibc/Dockerfile
Does that sound OK?

@eregon eregon force-pushed the fix-gcc85-conversion-warnings branch 2 times, most recently from 982568c to bb77ad0 Compare March 27, 2026 14:24
@eregon
Copy link
Copy Markdown
Member Author

eregon commented Mar 27, 2026

I added a CI job, before the fix it fails as expected:
https://github.com/ruby/prism/actions/runs/23650997866/job/68895672649?pr=4051
and after it passes.

Copy link
Copy Markdown
Collaborator

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're gonna do a whole thing here testing old GCCs, I don't want to just test this one specific version. Can we copy the stuff from ruby/ruby? Otherwise it feels very arbitrary.

https://github.com/ruby/ruby/blob/998e2f41c733589a30e940082c51ffa13762d901/.github/workflows/compilers.yml#L69-L97

@eregon
Copy link
Copy Markdown
Member Author

eregon commented Mar 27, 2026

@kddnewton That seems a lot more involved, could we merge this first?

If you don't like the CI job I can remove it, but it seems better than nothing.

For context, this is pretty much blocking a TruffleRuby release, as the release fails to build due to this.

I would say it is not arbitrary, it is the oldest free RHEL-like distribution which is not EOL yet, see https://endoflife.date/almalinux

@eregon eregon force-pushed the fix-gcc85-conversion-warnings branch from 7e6321e to 6069d67 Compare March 27, 2026 16:47
@eregon
Copy link
Copy Markdown
Member Author

eregon commented Mar 27, 2026

I have removed the CI job as I don't want that to block this fix.
I saved it on another branch: eregon@e1f1364

@Earlopain
Copy link
Copy Markdown
Collaborator

The amount of jobs that are tested in ruby is huge, I agree that replicating that isn't really desirable. I think it's good enough to happen just on sync, I don't think not having it has done much harm yet.

I'm ok with the added CI job. I don't really care which specific version it is or why it was chosen. It's within what ruby is testing and is more for the benefit of truffleruby than anything else.

@eregon
Copy link
Copy Markdown
Member Author

eregon commented Mar 27, 2026

I'm also thinking whether testing so many compilers wouldn't add much CI time, there are 11 and that's just gcc, there are 18+ more for clang at a glance.
The gcc ones appear to run in the same CI job though so maybe it isn't that slow?
It would also mean depending on ghcr.io/ruby/ruby-ci-image:$TAG.
The fact the gcc versions used there don't match what common Linux distributions use doesn't seem ideal (the 8.5 vs 8.4 mismatch mentioned here).
In any case that seems like a much bigger thing to figure out, hence why I removed the CI job from this PR.

@eregon eregon requested a review from kddnewton March 27, 2026 16:59
@kddnewton kddnewton merged commit ade5ef2 into ruby:main Mar 27, 2026
68 checks passed
@kddnewton
Copy link
Copy Markdown
Collaborator

I can't imagine it would add too much time, because I don't think we would run the tests, just ensure it compiles without warnings/errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compilation errors with gcc 8.5

3 participants